[No QA] feat(mfa): add the magic code step and registration decision to the state machine by dariusz-biela · Pull Request #355 · software-mansion-labs/expensify-app-fork

dariusz-biela · 2026-07-23T12:18:13Z

PR Stack

Part of the MFA flow to XState migration (#81197). The migration lands as a stack of vertical slices on the integration branch dariusz-81197-mfa-state-machine, and nothing reaches main until the final integration merge. The stack lives on the software-mansion-labs/expensify-app-fork remote, so this PR is opened there; its parent slice (#352) is already merged, so this PR targets the integration branch directly.

#	PR	Branch	Base
-	#345 (test harness)	`dariusz-biela/refactor/3ds/mfa-test-reachability-and-ui-walk`	`dariusz-81197-mfa-state-machine`
-	#346 (device check)	`dariusz-biela/feat/3ds/mfa-device-check-and-failure-screen`	`dariusz-81197-mfa-state-machine`
-	#352 (soft prompt)	`dariusz-biela/feat/3ds/mfa-soft-prompt`	`dariusz-81197-mfa-state-machine`
-	This PR	`dariusz-biela/feat/3ds/mfa-magic-code-and-registration-decision`	`dariusz-81197-mfa-state-machine`

Explanation of Change

Migrates the magic-code step and the registration decision from the legacy reducer into the MFA state machine, as the next vertical slice of the XState migration. The submitted code is exchanged with the backend for a registration challenge, so the whole loop (submit, reject, resend, retry) runs against the real round-trip.

In the app

After the device check, an account whose device credentials the server does not know gets a magic code emailed and sees the magic-code screen. A returning user whose credentials the server already knows skips it.
A rejected code shows the inline error without sending a new email; typing again or resubmitting clears it. Any other backend failure ends the flow on the failure outcome.
The resend link sends a fresh email as before, and is now also disabled while a submitted code is being verified.
The submit spinner now shows for accounts with 2FA enabled, where it previously stayed idle for the whole request.

In the code

A decidingRegistration state invokes a new checkLocalCredentials actor over areLocalCredentialsKnownToServer(accountID), added to the platform-resolved biometrics/operations modules (native reads the HSM key, web the stored passkeys, both compared against the server-known credential IDs). Anything short of a local key the server also knows means registration, so a missing key or a failed keystore read simply re-registers.
A magicCode parent state owns the screen and navigates to it on entry. Its awaitingValidateCode child waits for the code and is the only place a resend is accepted, so one fired mid-request is dropped; snapshotToState exposes that as canResendValidateCode, which drives the button instead of Onyx loading flags. The sibling requestingRegistrationChallenge child runs the exchange: an invalid code returns to awaitingValidateCode as an inline error, anything else ends the flow. The email request is an action on the decision transition, so it fires exactly once.
validateCode, continuableError, and registrationChallenge move from the legacy reducer into the machine context, and ValidateCodePage talks only to the machine-backed internal API.
The page derives the submit spinner from loadingForm === VALIDATE_CODE_FORM instead of AccountUtils.isValidateCodeFormSubmitting, which switches to the 2FA form key and so never matched here.
Tests: a transition spec for the decision and the magic-code loop, graph coverage of both decision branches and all challenge outcomes, a real-UI walk driving the loop through the rendered screen, and unit tests for areLocalCredentialsKnownToServer on both platforms.

Out of scope

readOnyxValueOnce, a one-shot Onyx read helper extracted from the soft-prompt actor and reused by the operations modules.
areLocalCredentialsKnownToServer now exists twice: the React-free operation the machine uses, and the biometrics hooks, which still back the Provider's start telemetry. The hook copies retire when those callers migrate.
The challenge's publicKeys stay unused until the registration slice, which also brings the key creation and RegisterAuthenticationKey.

Fixed Issues

$ Expensify#81197
PROPOSAL: N/A (internal engineering migration, part of the MFA XState refactor)

Tests

Run the app in dev on this branch, on an account whose device credentials are not registered with the server (a fresh dev account works).

Trigger an MFA scenario (for example Troubleshoot > Biometrics test). Verify that after the device check the magic-code screen appears and a magic code email arrives.
Submit an empty code, then a malformed one (fewer than 6 digits). Verify the inline validation error shows and no new email is sent.
Submit a well-formed but wrong 6-digit code. Verify "Incorrect or invalid security code. Please try again or request a new code." shows inline, the flow stays on the screen, no new email is sent, and the submit button spins while the code is verified (check this on an account with 2FA enabled too).
Start typing again. Verify the inline error clears.
Press "Didn't receive a magic code?" and verify a new email arrives and the countdown restarts. Then enter the received code and verify the flow continues past the screen (the soft prompt on first use on this device, the outcome otherwise).
Check the returning-user branch. The machine does not run the registration itself yet, so this branch cannot register a device on its own: check out main, complete the MFA registration there on this account and device, then come back to this branch and run the scenario again. Verify the magic-code screen is skipped and no email arrives.

Verify that no errors appear in the JS console

Offline tests

Reach the magic-code screen, then go offline. Verify the full-page offline blocking view covers the screen, the code cannot be submitted, and the resend link stays disabled (existing page behavior; the machine adds no new network handling).

QA Steps

[No QA] - internal stacked PR on the MFA XState integration branch. The flow is intentionally partial between slices and does not reach staging until the final integration merge.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

…machine After the device check the machine now decides whether registration is needed: a new checkLocalCredentials actor wraps a module-level areLocalCredentialsKnownToServer, so a returning user (or a flow that already carries a code) skips the magic-code screen entirely. The request-code side effect runs only on the decision transition, never on state entry, so the invalid-code retry loop cannot resend the email. validateCode and continuableError move from the legacy reducer into the machine context: an invalid code stays on the screen as a continuable error while any other rejection ends the flow through the outcome path. ValidateCodePage talks only to the internal API now. The true INVALID_VALIDATE_CODE round-trip becomes reachable in the registration slice; this slice wires and unit-tests the machine loop with mocked events.

The page clears the continuable error only when the user edits the code, so resubmitting the unchanged one carried the stale invalid-code error past the magic-code state, and the screen, which stays visible through the soft-prompt read, kept showing it while the flow had moved on. The submit transition now drops the error alongside storing the code, and a reject-then-resubmit spec pins the loop the walk cannot reach.

…tration decision validateCode is assigned only in the magic-code state, which never routes back to the decision, and every entry there goes through INIT, which resets the context. The guard could therefore never pass in a running flow; the spec exercising it only passed by seeding the code into the context by hand. The slice that loops registration back into the decision can reintroduce the guard together with the path that makes it reachable.

The submitted code now drives a real backend round-trip: a new requestingRegistrationChallenge state invokes an actor that wraps requestRegistrationChallenge, and the machine routes on the normalized result. A valid challenge lands in the machine context and the flow continues; an invalid code returns to the magic-code screen as the inline, continuable error; anything else ends the flow through the outcome path. This retires the mocked VALIDATE_CODE_REJECTED event and its walk exclusions, so the graph walk now drives the invalid-code retry loop through the real UI, including a dedicated journey. The legacy reducer's registrationChallenge field moves into the machine context with no remaining legacy consumers. The response's publicKeys stay unused until the registration slice reconciles local credentials.

…ured at flow start The registration decision needed the same credentials check the Provider already runs for start telemetry, and the machine actor duplicated the hook logic in the operations modules to get it, costing a second native keystore read per flow start. INIT now carries the captured localCredentialsKnownToServer flag, the decision becomes an eventless transition on it, and the operations copies and their suites go away, leaving the hooks as the single implementation. A keystore read failure now routes to registration instead of a fatal outcome, because the hooks resolve the check to false instead of rejecting; re-registration recovers such an account anyway. The UI walk grows stronger: the INIT executor seeds the biometrics hook mock, so the flag flows through the real Provider wiring, and both decision branches traverse via INIT fixture variants.

…hot captured at flow start" This reverts commit 550345f.

The resend button called requestValidateCodeAction directly from the view, bypassing the machine that owns every other send of the magic-code email. A new RESEND_VALIDATE_CODE event, accepted only while the magic-code screen waits for a code, makes the machine the single sender: a resend fired while the registration challenge request is in flight is dropped, and a resend also clears the stale inline invalid-code error. The view keeps only its UI-local cleanup and now disables the resend button on the request that actually loads during a resend (the `??` in the disable condition never reached its right-hand side).

… flight The machine drops a resend sent during the challenge request, but the button stayed pressable once the countdown expired, so a press cleared the input and restarted the countdown without a new email coming. The view now reads a flag derived from the machine snapshot, which cannot lag behind the state that decides whether the event is accepted, unlike the account loading state delivered through Onyx.

Upstream 6e87a8c started passing REGISTER_AUTHENTICATION_KEY on both sends of the magic-code email, and this slice moved both of them behind the machine's requestValidateCode action, so the action has to carry the reason code or the backend loses the context it was just given.

Upstream 2f096a5 renamed the user-facing magic-code keys to security-code. The page picked the new key up through the sync, the walk assertion still read the removed one and threw on every path that renders the inline error.

The Onyx bump that came with the sync widened the connect callback to a collection-aware conditional type, which no longer matches OnyxEntry. OnyxValue is the type Onyx resolves the callback to, and it is what tests/utils/getOnyxValue already uses.

Replace the continuableError context field and the CLEAR_CONTINUABLE_ERROR command event with an invalidCode substate of awaitingValidateCode. Every way out of the substate (typing, a resend, a new submission) clears the inline error by construction, so the three manual clear sites disappear. The VALIDATE_CODE_CHANGED event states what happened instead of commanding a context write, and the view reads the showsInvalidCodeError tag through snapshotToState. The stored MFAError payload had no consumers, so nothing replaces it.

Replace the hand-rolled done and error event shapes with XState's DoneActorEvent and ErrorActorEvent, keyed by actor id instead of by event type. The derived union lives in machine/machineEvents.ts so the machine module stays focused on the chart. Graph-traversal fixtures now hold one entry per actor, built by createActorEvents. Its non-empty return type carries the "at least one output variant" guarantee into the fixture table, and the keyed type pins each slot to that actor's own events. getTraversalEvents filters a single fixture list and keeps a separate branch for framework events that cannot be given a fixture at all. Also fixes flowActors.ts, which still called the previous createActorDoneEvent signature and did not compile.

jakubstec · 2026-07-30T13:17:59Z

+async function areLocalCredentialsKnownToServer(accountID: number, signal?: AbortSignal): Promise<boolean> {
+    const localCredentialID = await getLocalCredentialID(accountID);
+    if (!localCredentialID) {
+        return false;
+    }
+    const account = await readOnyxValueOnce(ONYXKEYS.ACCOUNT, signal);
+    return (mfaCredentialIDsSelector(account) ?? []).includes(localCredentialID);


NAB (native and web): we don't distinguish credentials that were not loaded yet from those non-existing (both undefined) - is there a chance, perhaps is there any chance that this actor runs before MFA data arrives from OpenApp? I think it's a small possibility for that, but if so, it would trigger registration flow unnecessarily

what's more, we could have false positive too (server credentials are gone but local credentials still exist), but only if multifactorAuthenticationPublicKeyIDs is not hydrated yet. it's worth to take under consideration in recovery slice

jakubstec · 2026-07-30T14:30:52Z

+const requestRegistrationChallengeActor = fromPromise<RequestRegistrationChallengeOutput, RequestRegistrationChallengeInput>(async ({input}) => {
+    const {challenge, httpStatusCode, reason, message} = await requestRegistrationChallenge(input.validateCode);
+    if (!isHttpSuccess(httpStatusCode) || !challenge) {
+        return {success: false, error: createMFAErrorFromApiResponse(httpStatusCode, reason, message)};
+    }
+    return {success: true, challenge};
+});



this actor doesn't accept an AbortSignal like other functions in this file, so it can resolve after the modal closes or the actor stops. Its finallyData unconditionally clears isLoading on ONYXKEYS.ACCOUNT, dismissing the modal and starting a new flow before the stale request finishes can let it clear the new flow's loading state causing race condition. I think it should skip the onyx write when aborted

jakubstec · 2026-07-30T14:41:43Z

                // Accepted only here: an INIT sent while the modal is open or still closing is
                // dropped rather than started on dirty state.
                INIT: {target: MFA_STATE.OPEN, actions: 'initFlow'},


i think it can cause race condition - if INIT is sent during closing (e.g. we are waiting for closeFallback) as the comment says, it's dropped without any notice (if (state.modalState !== MFA_STATE.CLOSED) return; in MultifactorAuthenticationMainContext.tsx L60)), but executeScenario could fire side effects

jakubstec · 2026-07-30T14:53:25Z

    types: {
        context: {} as MfaContext,
-        events: {} as MfaEvent,
+        events: {} as MfaMachineEvent,


from my understanding, we extended the machine's event type to include XState's internal actor-lifecycle events, but only to satisfy the graph-traversal tests. The production send() inherits that same widened type, so nothing stops real app code from constructing e.g send({type: 'xstate.done.actor.requestRegistrationChallenge', output: {success: true, challenge: fakeChallenge}}); directly and skipping the real backend check. Could we narrow the production back to MfaEvent and keep the wider typing test-only?

jakubstec · 2026-07-31T12:40:43Z

+function readOnyxValueOnce<TKey extends OnyxKey>(key: TKey, signal?: AbortSignal): Promise<OnyxValue<TKey>> {
+    return new Promise((resolve) => {
+        if (signal?.aborted) {
+            return;
+        }
+
+        let connection: Connection;
+        const disconnect = () => Onyx.disconnect(connection);
+
+        signal?.addEventListener('abort', disconnect, {once: true});
+        connection = Onyx.connectWithoutView({
+            key,
+            callback: (value) => {
+                signal?.removeEventListener('abort', disconnect);
+                disconnect();
+                resolve(value);
+            },
+        });
+    });
+}


NAB: maybe we should safely reject a promise after aborting instead of never resolving, as someone could misuse this helper outside of MFA (even though they shouldn't have)

jakubstec · 2026-07-31T12:53:19Z

-    const isValidateCodeFormSubmitting = AccountUtils.isValidateCodeFormSubmitting(account);
-    const shouldDisableResendCode = isOffline ?? account?.isLoading;
+    // The MFA registration challenge always uses VALIDATE_CODE_FORM, even when the account has 2FA enabled.
+    const isValidateCodeFormSubmitting = !!account?.isLoading && account.loadingForm === CONST.FORMS.VALIDATE_CODE_FORM;


i think this should depend on machine state/snapshot, not onyx as the flag can be stale/cleared

jakubstec · 2026-07-31T13:02:10Z

+                                onDone: [
+                                    {
+                                        guard: ({event}) => event.output.success,
+                                        target: SOFT_PROMPT_CHECK_TARGET,
+                                        actions: assign({registrationChallenge: ({event}) => (event.output.success ? event.output.challenge : undefined)}),
+                                    },


validateCode is only read once right there, further steps don't need it, but it's cleared only when entered CLOSED state. I think we should clear it before leaving this state, because it's sitting in context for the rest of the flow.

jakubstec · 2026-07-31T13:18:03Z

+            expect(screen.queryAllByTestId(TEST_ID.OUTCOME_SCREEN)).toHaveLength(0);
+            // A stored code means the flow re-entered this check from the magic-code screen, which
+            // stays visible while the read runs; a first pass runs behind the transparent initial screen.
+            if (state.context.validateCode === undefined) {


take under consideration this test, if we clear validateCode as suggested here https://github.com/software-mansion-labs/expensify-app-fork/pull/355/changes#r3690593440
because we use validateCode as a flag/indicator here

Gate areLocalCredentialsKnownToServer on HAS_LOADED_APP and IS_LOADING_APP so the check does not read ACCOUNT before OpenApp data arrives and start registration unnecessarily. Generalize readOnyxValueOnce into a predicate-based waitForOnyxValue and rename the module accordingly. On web, return early when there are no local passkeys so the gate never delays an already determined answer. The gate guarantees hydrated data, not fresh data. Reconciling credentials revoked while the app was closed stays with the recovery flow.

dariusz-biela mentioned this pull request Jul 24, 2026

[ECUK In-App 3DS] [App] Refactor MultifactorAuthenticationContextProvider - fix React rules violations and race conditions risks Expensify/App#81197

Open

5 tasks

dariusz-biela added 11 commits July 27, 2026 17:05

Revert "refactor(mfa): decide registration from the credentials snaps…

b0973f4

…hot captured at flow start" This reverts commit 550345f.

test(mfa): follow main's security-code translation rename

cda053e

Upstream 2f096a5 renamed the user-facing magic-code keys to security-code. The page picked the new key up through the sync, the walk assertion still read the removed one and threw on every path that renders the inline error.

dariusz-biela force-pushed the dariusz-biela/feat/3ds/mfa-magic-code-and-registration-decision branch from 7f77611 to 9e20ab4 Compare July 27, 2026 15:12

dariusz-biela added 16 commits July 27, 2026 17:31

test(mfa): align compiler handling for countdown mock

e8b1182

fix(mfa): derive resend availability from machine

fdee165

fix(mfa): show submit spinner for accounts with 2FA

80c66a7

Remove unused MFA test helper

e0c7247

Reuse MFA actor done event helper

d32b67e

refactor(mfa): nest magic-code request states

76d32be

fix(mfa): avoid stale loading state blocking resend

8b43487

test(mfa): cover registration challenge loading data

d88f31d

test(mfa): move validate-code loading test

20792aa

fix(mfa): cancel local credential reads

2bade7d

docs(mfa): clarify credential check ownership

73cbb2f

Improve MFA actor event type safety

a7bea77

refactor(mfa): rename idle input state

884788a

refactor(mfa): derive invalid-code error from state

c881945

jakubstec reviewed Jul 30, 2026

View reviewed changes

jakubstec reviewed Jul 31, 2026

View reviewed changes

dariusz-biela added 3 commits August 3, 2026 17:09

refactor(mfa): derive validate code submitting state from machine

7cf44c6

fix(mfa): clear validate code after challenge request

c8ac892

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[No QA] feat(mfa): add the magic code step and registration decision to the state machine - #355

[No QA] feat(mfa): add the magic code step and registration decision to the state machine#355
dariusz-biela wants to merge 30 commits into
dariusz-81197-mfa-state-machinefrom
dariusz-biela/feat/3ds/mfa-magic-code-and-registration-decision

dariusz-biela commented Jul 23, 2026 •

edited

Loading

Uh oh!

jakubstec Jul 30, 2026 •

edited

Loading

Uh oh!

jakubstec Jul 30, 2026

Uh oh!

jakubstec Jul 30, 2026

Uh oh!

jakubstec Jul 30, 2026

Uh oh!

jakubstec Jul 31, 2026

Uh oh!

jakubstec Jul 31, 2026

Uh oh!

jakubstec Jul 31, 2026

Uh oh!

jakubstec Jul 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

dariusz-biela commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Stack

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

jakubstec Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakubstec Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

jakubstec Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

jakubstec Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

jakubstec Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

jakubstec Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

jakubstec Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

jakubstec Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dariusz-biela commented Jul 23, 2026 •

edited

Loading

jakubstec Jul 30, 2026 •

edited

Loading